Skip to content

feat(inc): Add index to GroupOpenPeriod data jsonfield #97534

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Aug 11, 2025

Conversation

ceorourke
Copy link
Member

#96221 had to be reverted due to https://sentry.sentry.io/issues/6799376702 so we thought adding an index on GroupOpenPeriod's data jsonfield to the frequently looked up pending_incident_detector_id key would help speed things up.

@ceorourke ceorourke requested a review from a team as a code owner August 8, 2025 22:46
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Aug 8, 2025
@@ -73,6 +73,10 @@ class Meta:
indexes = (
# get all open periods since a certain date
models.Index(fields=("group", "date_started")),
models.Index(
models.F("data__pending_incident_detector_id"),
name="data__pend_inc_detector_id_idx",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

name is required and it's limited to 30 chars so I did the best I could here

Copy link
Contributor

github-actions bot commented Aug 8, 2025

This PR has a migration; here is the generated SQL for src/sentry/migrations/0966_groupopenperiod_data_pending_inc_detector_id_index.py

for 0966_groupopenperiod_data_pending_inc_detector_id_index in sentry

--
-- Create index data__pend_inc_detector_id_idx on F(data__pending_incident_detector_id) on model groupopenperiod
--
CREATE INDEX CONCURRENTLY "data__pend_inc_detector_id_idx" ON "sentry_groupopenperiod" ((("data" -> 'pending_incident_detector_id')));

# is a schema change, it's completely safe to run the operation after the code has deployed.
# Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment

is_post_deployment = False
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be post-deployment migration, as this table has >260M rows, and adding index will probably time out if we do it as in-deploy migration.

Copy link

codecov bot commented Aug 11, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff            @@
##           master   #97534    +/-   ##
========================================
  Coverage   80.62%   80.62%            
========================================
  Files        8560     8560            
  Lines      376889   377106   +217     
  Branches    24538    24538            
========================================
+ Hits       303870   304048   +178     
- Misses      72649    72688    +39     
  Partials      370      370            

Copy link
Member

@wedamija wedamija left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, hopefully this isn't too hard to create

@ceorourke ceorourke merged commit f0ef2cd into master Aug 11, 2025
65 checks passed
@ceorourke ceorourke deleted the ceorourke/groupopenperiod-index-jsonfield branch August 11, 2025 17:09
ceorourke added a commit that referenced this pull request Aug 12, 2025
Redo of #96221 after [adding an
index](#97534) to the
`GroupOpenPeriod`'s `data` column on `pending_incident_detector_id`
which should hopefully not result in [the
error](https://sentry.sentry.io/issues/6799376702/) anymore. There are
no changes to this PR from the original one.

---------

Co-authored-by: Snigdha Sharma <[email protected]>
andrewshie-sentry pushed a commit that referenced this pull request Aug 12, 2025
#96221 had to be reverted due to
https://sentry.sentry.io/issues/6799376702 so we thought adding an index
on `GroupOpenPeriod`'s `data` jsonfield to the frequently looked up
`pending_incident_detector_id` key would help speed things up.
andrewshie-sentry pushed a commit that referenced this pull request Aug 12, 2025
Redo of #96221 after [adding an
index](#97534) to the
`GroupOpenPeriod`'s `data` column on `pending_incident_detector_id`
which should hopefully not result in [the
error](https://sentry.sentry.io/issues/6799376702/) anymore. There are
no changes to this PR from the original one.

---------

Co-authored-by: Snigdha Sharma <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants